Review status: 0 of 27 files reviewed at latest revision, 1 unresolved discussion.

controller_client.py, line 1 [r1] (raw file):
This file is ugly and will disappear soon. It may be safely ignored.

Comments from Reviewable

Reviewed 18 of 27 files at r1.
Review status: 18 of 27 files reviewed at latest revision, 13 unresolved discussions.

requirements-fresh.txt, line 1 [r1] (raw file):
pynacl iba v fresh?

requirements.txt, line 8 [r1] (raw file):
Nerozumiem co robi records - vraj poskytuje raw SQL dotazy, ale pritom dependuje na sqlalchemy ktory ich pokial viem poskytuje tiez...

deadserver/server.py, line 14 [r1] (raw file):
Vyzera ze existuje DatagramRequestHandler. Zide sa ti?
Hm, ako tak pozeram, asi nie. Tak len pre uplnost.

deadserver/server.py, line 27 [r1] (raw file):
Ak spravne rozumiem rozdeleniu tried DeadServer a API, tak API je ta zaujimava (ktoru dostavaju handlery atd) a DeadServer je v podstate iba implementacny detail. Co takto ist este dalej a nahradit DeadServer fciou, v ktorej su config, db, server atd iba lokalne premenne?
Corollary: ak sa ti nepozdava mat config cisto lokalny, asi je vhodne odovzdat ho api.API, lebo teraz sa k nemu handlery aj tak nedostanu.

deadserver/server.py, line 29 [r1] (raw file):
Ach, preco je socketserver taky skaredy...
Mozno sa da vyhnut __init__-u a tejto partial havedi tak, ze triedu MessageHandler definujes priamo v tejto fcii, a handle() miesto "inner_self.api" pouzije "outer_self.api". (Resp lokalnu premennu ak ziaden outer_self nebude, vid vyssie.)

deadserver/utils.py, line 3 [r1] (raw file):
unused ;-)

deadserver/handlers/utils.py, line 7 [r1] (raw file):
Zvaz namiesto

@utils.unpack_indata_as(OpenRequest)
def handle(controller_id, data, api):

pouzit rovnako dlhe:

def handle(controller_id, data, api):
    data = utils.try_unpack(OpenRequest, data)

deadserver/handlers/utils.py, line 25 [r1] (raw file):
To by sa mozno dalo automaticky robit v api.py. Ked handler vrati nieco co nie je bytes (a asi None), pack()nes. Takze netreba vsetko dekorovat.

structparse/structdef.py, line 32 [r1] (raw file):
mozno mozes skratit na if rest:

structparse/structdef.py, line 38 [r1] (raw file):
As discussed: iba zavolaj vyssi konstruktor (bezo zmeny args a kwargs) a potom skontroluj ze fieldy su validne.

structparse/structdef.py, line 40 [r1] (raw file):
Hmm, takto sa objekty normalne nespravaju. id(list(x)) != id(x). A ked Foo je nejaky normalny namedtuple, tak Foo(Foo(10, 20, 30)) povie TypeError.
(Ale to len pre uplnost, toto aj tak zmizne.)

structparse/structdef.py, line 65 [r1] (raw file):
Ak chces, vyrob triedu fciou type(name, (nadtriedy...), dict(_fieldtypes=fieldtypes)).

structparse/types.py, line 1 [r1] (raw file):
As discussed, tieto triedy budu iba packovat a unpackovat a validovat, ale hodnoty budu primitivne.

Comments from Reviewable

Review status: 18 of 27 files reviewed at latest revision, 13 unresolved discussions.

requirements-fresh.txt, line 1 [r1] (raw file):
Uh, zabudla som freeznuť. Done.

requirements.txt, line 8 [r1] (raw file):
Má krajšiu API.

deadserver/server.py, line 14 [r1] (raw file):
Myslím, že som už niekedy usúdila, že mi je to takto pohodlnejšie.

deadserver/server.py, line 27 [r1] (raw file):
Odovzdávať config API som chcela len čo by v ňom začalo niečo byť. Ale môžem aj teraz.

Vymeniť DeadServer za fciu vlastne môžem. Tak teda done.

deadserver/server.py, line 29 [r1] (raw file):
Done. Lepšie.

deadserver/utils.py, line 3 [r1] (raw file):
Vlastne hej, keďže som urvala structparse samostatne. Tým pádom celé utils je preč.

deadserver/handlers/utils.py, line 7 [r1] (raw file):
Nie vždy chceš unpackovať (napr. echotest). Prišlo mi krajšie mať to tam explicitne. Môžem to for now nechať tak a snáď nezabudnúť na túto alternatívu? :D

deadserver/handlers/utils.py, line 25 [r1] (raw file):
Dalo, tu som to chcela mať symetrické s tým unpack()om, ale asi je to hlúpe...

structparse/structdef.py, line 32 [r1] (raw file):
Done.

structparse/structdef.py, line 38 [r1] (raw file):
V skutočnosti by som to chcela prerobiť na iba classmethods/objekty/stuff a riešiť všetko až v pack() a tým pádom vôbec nechytať __new__. Ak sa neurazíš, teraz to nechám tak a budem kódiť funkcionalitu (deadline na bakalárku je o mesiac! :D) a napíšem si issue, že to chcem prerobiť na hentak.

structparse/structdef.py, line 40 [r1] (raw file):
OK, máš pravdu. Toto správanie vzniklo lebo niekde som chvíľu nemala jasno v tom, či je niečo už skonvertované alebo nie. Ešte že to chcem celé zapáliť.

structparse/structdef.py, line 65 [r1] (raw file):
Hm, to nepoznám :D ďakujem :D

structparse/types.py, line 1 [r1] (raw file):
Presne.

Comments from Reviewable

Review status: 12 of 26 files reviewed at latest revision, 12 unresolved discussions.

deadserver/handlers/utils.py, line 7 [r1] (raw file):
Hm, môj predošlý komentár nedáva zmysel :D ignoruj ho :D

Comments from Reviewable

Review status: 12 of 26 files reviewed at latest revision, 12 unresolved discussions.

deadserver/handlers/utils.py, line 7 [r1] (raw file):
Ale podľa mňa má svoje čaro povedať, že handler vždy žerie bytes a vracia bytes, as opposed to "žerie bytes a vracia bytes, ibaže by struct". A tak by som chcela vždy vracať bytes. A zároveň mi príde estetickejšie odekorovať to ako na začiatku mať unpackovanie a na konci packovanie, lebo to to nezaujímavé castovanie vyberie von z toho, kde je logika fcie.

Comments from Reviewable

Reviewed 6 of 27 files at r1, 9 of 9 files at r2.
Review status: all files reviewed at latest revision, 19 unresolved discussions.

requirements.txt, line 8 [r1] (raw file):
Eh, budiz.
Mozno by som ta casom presviedcal spravit to nejako inak, ale zatial je to takto fajn.

deadserver/api.py, line 21 [r2] (raw file):
vacsina style guidov (a mozno Guidov?) by preferovala jedno z:

dlha_fcia(
    arg, arg,
    arg, arg)
dlha_fcia(arg, arg,
          arg, arg)

also preco je akurat get_key kwarg? (a vyssie preco akurat api je kwarg?)

deadserver/api.py, line 33 [r2] (raw file):
co ja viem ci krajsie api :p
nechces nejaku kniznicu co umoznuje Controller.get_one(id=protocol.id2str(id))?
(admittedly, aj sqlalchemy neni nic moc v tomto... eh, zatial nechaj tak a mozno neskor nieco lepsie)

deadserver/api.py, line 39 [r2] (raw file):
request je tuto bytes? nechces repr() alebo tak? (ak print spravi automaticky spravnu vec tak netreba)

deadserver/protocol.py, line 4 [r2] (raw file):
existuje controller_api stale?

deadserver/protocol.py, line 36 [r2] (raw file):
Mozno z types importni vsetko potrebne - takto raz mas MsgTypea raz types.Tail. Ci to je naschval?

deadserver/protocol.py, line 75 [r2] (raw file):

• preco nie Packet.unpack?
• hmm. mozno by sa dokonca cely PacketHeader dal zrusit a inlinnut v Packet. tam kde teraz mas hdr, request_header atd by bol cely Packet. co by nemalo nicomu vadit. ale neviem ci dobry napad.

deadserver/protocol.py, line 103 [r2] (raw file):
Mozno ; -> \n

deadserver/server.py, line 8 [r2] (raw file):
Nit: functools uz netreba

deadserver/handlers/init.py, line 26 [r2] (raw file):
buaa megadlhy komentar. :D
neni toto cele vyrazne komplikovanejsie ako musi byt? in the end aj tak potrebujes niekde subor co obsahuje zoznam vsetkych handlerov. tak co takto rovno:

from . import echotest, open, foo, bar
handlers = {
    MsgType.ECHOTEST: echotest.handler,
    MsgType.OPEN: open.handler,
    MsgType.FOO: foo.handler,
    MsgType.BAR: bar.handler,
}

Alebo ak sa ti nepaci ze MsgType je tuto, kazdy modul moze definovat fciu handler a konstantu msgtype a tu spravis povedzme...

from . import echotest, open, foo, bar
handlers = { m.msgtype: m.handler for m in [echotest, open, foo, bar] }

deadserver/handlers/defs.py, line 12 [r2] (raw file):
Mozno priamo zverejnit dict miesto tejto fcie?
(ale tiez vid __init__.py komentare)

deadserver/handlers/utils.py, line 7 [r1] (raw file):
Huh?? Ahaaaa. To odpovedas na moj druhy komentar o tom ze fcia moze vracat bud bytes alebo struct. OK, kludne nech vzdy vracia bytes...

Dekoratory vs unpack+pack vnutri:
povedal by som ze unpackovanie je sucastou "logiky fcie" ;-) a logika echotestu je ze neunpackujes.
a tiez je jasnejsie aky je funkcny prototyp a ze caller ma fciu naozaj zavolat s bytes. s dekoratorom je to trochu "najprv sa stane nejaka magia a potom zrazu mam struct".
mozes nechat aj dekoratory ak chces, ale podobne ako s metaclassami apod je tam isty tradeoff, ze ci "zvysena magickost" stoji za pridanu hodnotu. tu konkretne by som ich ja osobne asi nepouzil.

structparse/structdef.py, line 38 [r1] (raw file):
Hej, to bola jedna z uvazovanych variant. Prides o skoru validaciu (ked sa validacia tiez deje az v pack()), ale zaobides sa bez __new__. (A validacia sa teoreticky da doplnit neskor...)
Eh, moc sa mi nepozdava nutnost stale konvertovat primitivne typy. Kto ma vsetky tie .val reviewovat :p Isto to nechces prepisat skorej? Nemalo by to byt az taka velka zmena, a cim neskor tym vacsia. Ale ak nie tak budiz...

structparse/structdef.py, line 40 [r1] (raw file):
Ked uz neprepisujes tak aspon fixni skaredu verziu :p
Ale imho este lepsie prepisat...

structparse/tests/test_struct.py, line 42 [r2] (raw file):
strct, vazne? :p
Mozno radsej unpacked?

Comments from Reviewable